Skip to content

fix: disable wifi when playing songs on buzzer#19

Merged
bharvey88 merged 5 commits into
ApolloAutomation:betafrom
firstof9:patch-1
Dec 16, 2025
Merged

fix: disable wifi when playing songs on buzzer#19
bharvey88 merged 5 commits into
ApolloAutomation:betafrom
firstof9:patch-1

Conversation

@firstof9

@firstof9 firstof9 commented Dec 16, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Version:

What does this implement/fix?

Stops ESP chip from resetting when playing songs at 50% on the buzzer.

Types of changes

  • Bugfix (fixed change that fixes an issue)
  • New feature (thanks!)
  • Breaking change (repair/feature that breaks existing functionality)
  • Dependency Update - Does not publish
  • Other - Does not publish
  • Website of github readme file update - Does not publish
  • Github workflows - Does not publish

Checklist / Checklijst:

  • The code change has been tested and works locally
  • The code change has not yet been tested

If user-visible functionality or configuration variables are added/modified:

  • Added/updated documentation for the web page

Summary by CodeRabbit

  • New Features
    • Added four new song playback scripts that run RTTTL tunes with synchronized light effects (brightness and RGB) across devices.
    • Each script temporarily manages Wi‑Fi during playback, waits for the tune to finish (~30s), turns lights off, and then invokes the existing sleep flow.

✏️ Tip: You can customize this high-level summary in your review settings.

@firstof9
fix: disable wifi when playing songs on buzzer
48e4f75 
Stops ESP chip from resetting when playing songs at 50% on the buzzer.
@coderabbitai

coderabbitai Bot commented Dec 16, 2025 •
edited
Loading

Copy link
Copy Markdown

Walkthrough

Adds four new playback scripts to the ESPHome H-2 YAML that sequence light actions, toggle Wi‑Fi around RTTTL playback, wait, turn off lights, and call the existing sleep-check script.

Changes

Cohort / File(s) Summary
Script Additions
Integrations/ESPHome/H-2.yaml		 Added four new scripts (play_song_1, play_song_2, play_song_3, play_song_4) that orchestrate RGB/logo light on/off and effects, disable/enable Wi‑Fi for RTTTL playback, wait ~30s, then run the existing ShouldSleep flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single YAML file with repetitive, consistent script additions.
  • Checkpoints: confirm RTTTL song-state lambdas referenced exist and Wi‑Fi toggle actions are correct; ensure naming doesn't conflict with existing scripts.

Poem

🐰 I hopped in code with a joyful cheer,
Four little tunes now twinkle here,
Lights blink, Wi‑Fi hushes, a melody hums,
Then cozy sleep—soft drumbeat drums. ✨🎶

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: adding scripts to disable WiFi when playing songs, which matches the PR objectives of preventing ESP chip reset during buzzer playback.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2a4ff5 and 93bfb1a.

📒 Files selected for processing (1)
  • Integrations/ESPHome/H-2.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Integrations/ESPHome/H-2.yaml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed Dec 16, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48e4f75 and e2a4ff5.

⛔ Files ignored due to path filters (1)
  • .DS_Store is excluded by !**/.DS_Store
📒 Files selected for processing (1)
  • Integrations/ESPHome/H-2.yaml (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
Integrations/ESPHome/H-2.yaml

[error] 257-257: duplication of key "script" in mapping

(key-duplicates)

🔇 Additional comments (1)
Integrations/ESPHome/H-2.yaml (1)

258-332: rtttl.play is non-blocking; WiFi will re-enable before playback completes.

The fix disables WiFi before playback but re-enables it immediately after the rtttl.play action returns. Since rtttl.play provides an on_finished_playback callback, this action is non-blocking and returns before the song finishes. WiFi will be re-enabled mid-playback, leaving the reset vulnerability unresolved.

Use the on_finished_playback callback instead: defer wifi.enable and subsequent actions until after playback completes.

Comment thread Integrations/ESPHome/H-2.yaml Outdated
@firstof9 firstof9 marked this pull request as draft December 16, 2025 19:04
@firstof9 firstof9 marked this pull request as ready for review December 16, 2025 22:56
@bharvey88

bharvey88 commented Dec 16, 2025

Copy link
Copy Markdown
Contributor

seems to work merging then doing more testing thanks man!

@bharvey88 bharvey88 merged commit 35c2249 into ApolloAutomation:beta Dec 16, 2025
7 of 8 checks passed
@firstof9 firstof9 deleted the patch-1 branch December 17, 2025 15:33
@coderabbitai coderabbitai Bot mentioned this pull request Dec 17, 2025
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@firstof9 @bharvey88